[Feat] Add XRPL #67

tanut32039 · 2026-01-13T04:28:24Z

Fixed: #XXXX

Implementation details

refactor some parts to be compatible for next chain type (ex. xrpl native, sol)

Please ensure the following requirements are met before submitting a pull request:

The pull request is targeted against the correct target branch
The pull request is linked to an issue with appropriate discussion and an accepted design OR is linked to a spec that describes the work.
The pull request includes a description of the implementation/work done in detail.
The pull request includes any and all appropriate unit/integration tests
You have added a relevant changelog entry to CHANGELOG_UNRELEASED.md
You have re-reviewed the files affected by the pull request (e.g. using the Files changed tab in the Github PR explorer)

Copilot

Pull request overview

This pull request refactors the wallet and key management architecture to support multiple chain types beyond EVM (e.g., XRPL native, Solana). The refactoring makes the codebase more modular and chain-agnostic by extracting common wallet operations into a shared chains package.

Changes:

Changed SavePrivateKey interface from accepting *ecdsa.PrivateKey to accepting string for chain-agnostic key handling
Moved key management functions (AddKeyByPrivateKey, DeleteKey, ExportPrivateKey, ListKeys, ShowKey, AddRemoteSignerKey) from chain provider implementations to a common chains package
Updated all tests and mocks to reflect the new architecture

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
relayer/wallet/wallet.go	Changed SavePrivateKey signature to accept string instead of *ecdsa.PrivateKey
relayer/wallet/geth/wallet.go	Updated implementation to convert hex string to ECDSA key internally
relayer/wallet/geth/wallet_test.go	Updated tests to pass hex-encoded private keys
relayer/chains/keys.go	New file containing chain-agnostic key management functions
relayer/chains/signer.go	New file containing LoadSigners helper function
relayer/chains/provider.go	Removed key management methods from KeyProvider interface
relayer/chains/evm/keys.go	Simplified to only handle mnemonic-based key generation
relayer/chains/evm/signer.go	Simplified to delegate to chains.LoadSigners
relayer/chains/evm/keys_test.go	Updated tests to use new chains package functions
relayer/chains/signer_test.go	Moved and updated test, changed package name
relayer/app.go	Added getWallet helper and updated to use chains package functions
relayer/app_test.go	Updated mock expectations to work with new architecture
internal/relayertest/mocks/wallet.go	Updated mock SavePrivateKey signature
internal/relayertest/mocks/chain_provider.go	Removed key management methods from mock

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

relayer/wallet/geth/wallet_test.go

relayer/chains/keys.go

relayer/chains/provider.go

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

relayer/chains/signer.go

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

relayer/chains/provider.go

Copilot

Pull request overview

Copilot reviewed 45 out of 46 changed files in this pull request and generated 11 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

relayer/chains/xrpl/provider.go

relayer/wallet/xrpl/wallet.go

relayer/tunnel_relayer.go

relayer/chains/xrpl/utils.go

relayer/chains/xrpl/provider.go

cmd/flags.go

go.mod

relayer/chains/xrpl/config.go

relayer/wallet/xrpl/wallet.go

Copilot

Pull request overview

Copilot reviewed 45 out of 46 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (2)

relayer/chains/xrpl/provider.go:300

The XRPL implementation lacks test coverage. While EVM chains have comprehensive tests (keys_test.go, provider_test.go, utils_test.go), XRPL has no test files. At minimum, tests should be added for critical functionality like wallet operations (SaveBySecret, SaveByMnemonic, DeleteKey), provider operations (RelayPacket, QueryBalance), and utility functions (stringToHex, parseAssetsFromSignal).
relayer/wallet/xrpl/wallet.go:270
The XRPL wallet implementation lacks test coverage. Critical wallet operations like SaveBySecret, SaveByMnemonic, SaveRemoteSignerKey, and DeleteKey should have tests similar to the GethWallet test suite to ensure they work correctly and handle edge cases properly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

relayer/tunnel_relayer.go

relayer/wallet/xrpl/wallet.go

relayer/chains/xrpl/provider.go

relayer/chains/xrpl/utils.go

relayer/tunnel_relayer.go

relayer/wallet/xrpl/wallet.go

relayer/chains/xrpl/provider.go

Copilot

Pull request overview

Copilot reviewed 70 out of 71 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

relayer/app_test.go:447

This test uses time.Now().Unix() twice when constructing the expected and returned packets; those calls can differ by a second, making the equality assertion flaky. Capture the timestamp once (e.g., now := time.Now().Unix()) and reuse it for both packets (or assert on fields excluding CreatedAt).

	// Create the expected Packet object
	tunnelPacketBandInfo := bandtypes.NewPacket(
		1,
		1,
		signalPrices,
		signingInfo,
		nil,
		time.Now().Unix(),
	)

	// Set up the mock expectation
	s.client.EXPECT().
		GetTunnelPacket(gomock.Any(), uint64(1), uint64(1)).
		Return(tunnelPacketBandInfo, nil)

	// Call the function under test
	packet, err := s.app.QueryTunnelPacketInfo(context.Background(), 1, 1)

	// Create the expected packet structure for comparison
	expected := bandtypes.NewPacket(1, 1, signalPrices, signingInfo, nil, time.Now().Unix())

	// Assertions
	s.Require().NoError(err)
	s.Require().Equal(expected, packet)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

relayer/wallet/xrpl/remote_signer.go

relayer/chains/xrpl/client.go

cmd/keys.go

README.md

relayer/wallet/xrpl/remote_signer.go

Copilot

Pull request overview

Copilot reviewed 69 out of 70 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T11:03:10Z

scripts/mockgen.sh

+$mockgen_cmd -source=relayer/chains/xrpl/client.go -mock_names Client=MockXRPLClient -package mocks -destination internal/relayertest/mocks/chain_xrpl_client.go
+


There is an extra trailing blank line added at the end of the script. Consider removing it to keep generated diffs clean and avoid linting/formatting noise in shell scripts.

Copilot · 2026-02-12T11:03:10Z

cmd/keys.go

 	cmd.Flags().String(flagPrivateKey, "", "add key with the given private key")
+	cmd.Flags().String(flagFamilySeed, "", "add key with the given family seed")
 	cmd.Flags().String(flagMnemonic, "", "add key with the given mnemonic")
 	cmd.Flags().Uint64(flagCoinType, defaultCoinType, "coin type number for HD derivation")
 	cmd.Flags().Uint64(flagWalletAccount, 0, "account number in the HD derivation path")


The --coin-type flag defaults to defaultCoinType (60), but this PR introduces XRPL support and App.AddKeyByMnemonic now rejects coin types other than 144 for XRPL. As a result, keys add <xrpl-chain> <key> without explicitly setting --coin-type 144 will fail. Consider defaulting coin type based on the target chain type (after InitTargetChain) or overriding the default to 144 when the chain is XRPL (and update the prompt/flag help accordingly).

Copilot · 2026-02-12T11:03:11Z

relayer/chains/xrpl/utils.go

+func Uint64StrToHexStr(uint64Str string) (string, error) {
+	n := new(big.Int)
+	n, ok := n.SetString(uint64Str, 10)
+	if !ok {
+		return "", fmt.Errorf("invalid numeric string: %s", uint64Str)
+	}
+
+	return fmt.Sprintf("%016X", n), nil


Uint64StrToHexStr formats a big.Int with %016X without enforcing that the parsed value is within uint64 (0..2^64-1). If the input is negative or exceeds uint64, this will produce an invalid-width hex string that can break XRPL binary encoding. Consider validating the value fits in uint64 and returning an error otherwise.

Copilot · 2026-02-12T11:03:11Z

relayer/wallet/geth/wallet.go

+// SaveBySecret imports the ECDSA key into the keystore and writes its signer record.
+func (w *GethWallet) SaveByPrivateKey(name string, secret string) (addr string, err error) {
 	// check if the key name exists


The doc comment says "SaveBySecret" but the method name is SaveByPrivateKey. Please align the comment with the exported API to avoid confusion in generated docs and when grepping for method behavior.

tanut32039 requested review from Marca23 and RogerKSI January 13, 2026 04:28

RogerKSI requested a review from Copilot January 13, 2026 07:00

Copilot started reviewing on behalf of RogerKSI January 13, 2026 07:01 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

relayer/wallet/geth/wallet_test.go Show resolved Hide resolved

relayer/chains/keys.go Outdated Show resolved Hide resolved

RogerKSI reviewed Jan 13, 2026

View reviewed changes

relayer/chains/provider.go Show resolved Hide resolved

RogerKSI requested review from RogerKSI and Copilot January 14, 2026 08:37

Copilot started reviewing on behalf of RogerKSI January 14, 2026 08:38 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

relayer/chains/signer.go Show resolved Hide resolved

RogerKSI requested a review from Copilot January 14, 2026 10:09

Copilot started reviewing on behalf of RogerKSI January 14, 2026 10:10 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

RogerKSI reviewed Jan 15, 2026

View reviewed changes

relayer/chains/provider.go Outdated Show resolved Hide resolved

tanut32039 force-pushed the refactor-for-next-chain branch from 769b441 to 81fddd7 Compare January 26, 2026 09:23

tanut32039 changed the title ~~[Feat] refactor for next chain~~ [Feat] Add XRPL Jan 30, 2026

RogerKSI requested review from RogerKSI and Copilot January 30, 2026 09:49

Copilot started reviewing on behalf of RogerKSI January 30, 2026 09:49 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

RogerKSI requested a review from Copilot February 3, 2026 07:41

Copilot started reviewing on behalf of RogerKSI February 3, 2026 07:41 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

Tanut Lertwarachai added 7 commits February 12, 2026 13:23

refactor for next chain

c1282ce

fix copilot comment

3b2095c

fix wrong test result

702a869

add xrpl

6637368

refactor

ffe4457

add test

d66cf75

fix metrics

98e393e

Tanut Lertwarachai added 2 commits February 12, 2026 13:23

fix copilot comment

a663316

integrate with fkms

b65571a

tanut32039 force-pushed the refactor-for-next-chain branch from 1dc46f7 to b65571a Compare February 12, 2026 06:23

Tanut Lertwarachai added 3 commits February 12, 2026 14:05

add some unit tests

a4d5e6b

add wallet unit test

4c14139

fix lint

d1f39ff

RogerKSI requested a review from Copilot February 12, 2026 07:27

Copilot started reviewing on behalf of RogerKSI February 12, 2026 07:27 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

fix copilot comment

c9afe1e

RogerKSI requested a review from Copilot February 12, 2026 10:49

Copilot started reviewing on behalf of RogerKSI February 12, 2026 10:50 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

		$mockgen_cmd -source=relayer/chains/xrpl/client.go -mock_names Client=MockXRPLClient -package mocks -destination internal/relayertest/mocks/chain_xrpl_client.go

[Feat] Add XRPL #67

Are you sure you want to change the base?

[Feat] Add XRPL #67

Uh oh!

Conversation

tanut32039 commented Jan 13, 2026

Implementation details

Please ensure the following requirements are met before submitting a pull request:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!